Skip to content

Conversation

PiotrDuz
Copy link
Contributor

@PiotrDuz PiotrDuz commented Oct 6, 2025

Closes #10446

Makes stopRenew() method thread safe.

Signed-off-by: PiotrDuz [email protected]

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signed-off-by: PiotrDuz <[email protected]>

Please, consider to use your legal name and real email.
For DCO requirements.

@PiotrDuz
Copy link
Contributor Author

PiotrDuz commented Oct 9, 2025

Hey, So I wouldn't like to post my personal data open in the web.
The name is a shortcut of my real name, so there is some association if it is really required :)

And all the details match ym account , also I sign it from the same account that authored the commit - then there is little room to claim that author has not agreed on the terms. Would it be ok ? :>

@artembilan
Copy link
Member

OK, @PiotrDuz ,

Got an answer from our management.
So, as long as the email is valid @users.noreply.github.com, we even don't need your real name.
If legal concern arises, we would reach your through GitHub support. 😄

Now I can review and merge your PR.

Thank you for your patience!

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And feel free to add your name to the @author list of the affected class.

Thank you!

return res;
}

/* This method can be called by more than 1 thread, thus we need to make sure that it is safe*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this comment here.
Because it is obvious from the method and property scope that it is a bug with concurrent access.
Therefore, we follow "local variable extraction" pattern everywhere in similar situations.

Please, remove this comment altogether.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


/home/runner/work/spring-integration/spring-integration/spring-integration-redis/src/test/java/org/springframework/integration/redis/outbound/RedisQueueOutboundChannelAdapterTests.java:134: error: cannot find symbol
		handler.setSerializer(new Jackson3JsonRedisSerializer<Object>(Object.class));
		                          ^
  symbol:   class Jackson3JsonRedisSerializer
  location: class RedisQueueOutboundChannelAdapterTests

Please, consider to rebase your branch to the latest upstream/main.
We have the fix for that Spring Data change in already.

@artembilan
Copy link
Member

Any updates, please?
We have release on next Tuesday, so would be great to make this in.
If you cannot update, no worries: we fix it at last moment before release.
Thanks

@PiotrDuz
Copy link
Contributor Author

Hey @artembilan , sorry for the delay.
I have removed the comment and rebased on the newest upstream main branch.
Regards!

* @author Youbin Wu
* @author Michal Domagala
* @author Severin Kistler
* @author PiotrDuz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, only real name can go here.
This part is for the credability: when some one reads our code, they would see who did changes to the class.
When it is nick-name like this there is not too much to acknowledge the person.
Therefore, if you don't want to share your real name, then you might not interested in the credit, thus no need in this @author in the class.

Thanks for understanding!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, this has been changes. No worries ;)

Remove the @author line 

Signed-off-by: PiotrDuz <[email protected]>
@artembilan artembilan merged commit 17beaef into spring-projects:main Oct 20, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RedisLockRegistry stopRenew not thread safe

2 participants